From: Ian Campbell Date: Tue, 24 May 2011 14:57:24 +0000 (+0100) Subject: libxl: add statup checks to libxl__wait_for_device_model X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~10319 X-Git-Url: https://dgit.raspbian.org/%22http://www.example.com/cgi/success//%22http:/www.example.com/cgi/success/?a=commitdiff_plain;h=3b6eaa3e341771827a9eca6c3eb8ee72593a3f56;p=xen.git libxl: add statup checks to libxl__wait_for_device_model When the device model is starting up push checks for spawn failure down into libxl__wait_for_device_model, allowing us to fail more quickly when the device model fails to start (e.g. due to a missing library or an early setup error etc). In order to allow the select loop in libxl__wait_for_device_model to wake when the child dies add pipe between the parent and the intermediate process which the intermediate process can use to signal the parent. Signed-off-by: Ian Campbell Acked-by: Ian Jackson Committed-by: Ian Jackson --- diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index ccf6518a89..a096d60f3d 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -522,7 +522,8 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid) state = libxl__xs_read(&gc, XBT_NULL, path); if (state != NULL && !strcmp(state, "paused")) { libxl__xs_write(&gc, XBT_NULL, libxl__sprintf(&gc, "/local/domain/0/device-model/%d/command", domid), "continue"); - libxl__wait_for_device_model(&gc, domid, "running", NULL, NULL); + libxl__wait_for_device_model(&gc, domid, "running", + NULL, NULL, NULL); } } ret = xc_domain_unpause(ctx->xch, domid); diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index 5d85822620..a3826bdc1c 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -408,6 +408,7 @@ out: int libxl__wait_for_device_model(libxl__gc *gc, uint32_t domid, char *state, + libxl__device_model_starting *starting, int (*check_callback)(libxl__gc *gc, uint32_t domid, const char *state, @@ -437,7 +438,17 @@ int libxl__wait_for_device_model(libxl__gc *gc, tv.tv_sec = LIBXL_DEVICE_MODEL_START_TIMEOUT; tv.tv_usec = 0; nfds = xs_fileno(xsh) + 1; + if (starting && starting->for_spawn->fd > xs_fileno(xsh)) + nfds = starting->for_spawn->fd + 1; + while (rc > 0 || (!rc && tv.tv_sec > 0)) { + if ( starting ) { + rc = libxl__spawn_check(gc, starting->for_spawn); + if ( rc ) { + rc = -1; + goto err_died; + } + } p = xs_read(xsh, XBT_NULL, path, &len); if ( NULL == p ) goto again; @@ -459,15 +470,24 @@ again: free(p); FD_ZERO(&rfds); FD_SET(xs_fileno(xsh), &rfds); + if (starting) + FD_SET(starting->for_spawn->fd, &rfds); rc = select(nfds, &rfds, NULL, NULL, &tv); if (rc > 0) { - l = xs_read_watch(xsh, &num); - if (l != NULL) - free(l); - else - goto again; + if (FD_ISSET(xs_fileno(xsh), &rfds)) { + l = xs_read_watch(xsh, &num); + if (l != NULL) + free(l); + else + goto again; + } + if (starting && FD_ISSET(starting->for_spawn->fd, &rfds)) { + unsigned char dummy; + read(starting->for_spawn->fd, &dummy, sizeof(dummy)); + } } } +err_died: xs_unwatch(xsh, path, path); xs_daemon_close(xsh); LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model not ready"); diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index a71d7cfcbe..7a81cbfe26 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -855,14 +855,12 @@ static int detach_device_model(libxl__gc *gc, return rc; } - int libxl__confirm_device_model_startup(libxl__gc *gc, libxl__device_model_starting *starting) { - int problem = libxl__wait_for_device_model(gc, starting->domid, "running", NULL, NULL); int detach; - if ( !problem ) - problem = libxl__spawn_check(gc, starting->for_spawn); + int problem = libxl__wait_for_device_model(gc, starting->domid, "running", + starting, NULL, NULL); detach = detach_device_model(gc, starting); return problem ? problem : detach; } diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 667b20b0eb..f6bca7efb7 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -543,7 +543,7 @@ int libxl__domain_save_device_model(libxl__gc *gc, uint32_t domid, int fd) LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Saving device model state to %s", filename); libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid), "save"); - libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL); + libxl__wait_for_device_model(gc, domid, "paused", NULL, NULL, NULL); if (stat(filename, &st) < 0) { diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c index 70b54259a6..ce6e93f4f0 100644 --- a/tools/libxl/libxl_exec.c +++ b/tools/libxl/libxl_exec.c @@ -26,6 +26,7 @@ #include #include #include /* for SIGKILL */ +#include #include "libxl.h" #include "libxl_internal.h" @@ -91,6 +92,22 @@ void libxl_report_child_exitstatus(libxl_ctx *ctx, } } +static int libxl__set_fd_flag(libxl__gc *gc, int fd, int flag) +{ + int flags; + + flags = fcntl(fd, F_GETFL); + if (flags == -1) + return ERROR_FAIL; + + flags |= flag; + + if (fcntl(fd, F_SETFL, flags) == -1) + return ERROR_FAIL; + + return 0; +} + int libxl__spawn_spawn(libxl__gc *gc, libxl__spawn_starting *for_spawn, const char *what, @@ -100,22 +117,33 @@ int libxl__spawn_spawn(libxl__gc *gc, { libxl_ctx *ctx = libxl__gc_owner(gc); pid_t child, got; - int status; + int status, rc; pid_t intermediate; + int pipes[2]; + unsigned char dummy = 0; if (for_spawn) { for_spawn->what = strdup(what); if (!for_spawn->what) return ERROR_NOMEM; + + if (libxl_pipe(ctx, pipes) < 0) + goto err_parent; + if (libxl__set_fd_flag(gc, pipes[0], O_NONBLOCK) < 0 || + libxl__set_fd_flag(gc, pipes[1], O_NONBLOCK) < 0) + goto err_parent_pipes; } intermediate = libxl_fork(ctx); - if (intermediate ==-1) { - if (for_spawn) free(for_spawn->what); - return ERROR_FAIL; - } + if (intermediate ==-1) + goto err_parent_pipes; + if (intermediate) { /* parent */ - if (for_spawn) for_spawn->intermediate = intermediate; + if (for_spawn) { + for_spawn->intermediate = intermediate; + for_spawn->fd = pipes[0]; + close(pipes[1]); + } return 1; } @@ -124,8 +152,10 @@ int libxl__spawn_spawn(libxl__gc *gc, child = fork(); if (child == -1) exit(255); - if (!child) + if (!child) { + if (for_spawn) close(pipes[1]); return 0; /* caller runs child code */ + } intermediate_hook(hook_data, child); @@ -134,9 +164,23 @@ int libxl__spawn_spawn(libxl__gc *gc, got = call_waitpid(ctx->waitpid_instead, child, &status, 0); assert(got == child); - _exit(WIFEXITED(status) ? WEXITSTATUS(status) : + rc = (WIFEXITED(status) ? WEXITSTATUS(status) : WIFSIGNALED(status) && WTERMSIG(status) < 127 ? WTERMSIG(status)+128 : -1); + if (for_spawn) + write(pipes[1], &dummy, sizeof(dummy)); + _exit(rc); + + err_parent_pipes: + if (for_spawn) { + close(pipes[0]); + close(pipes[1]); + } + + err_parent: + if (for_spawn) free(for_spawn->what); + + return ERROR_FAIL; } static void report_spawn_intermediate_status(libxl__gc *gc, diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 9c77f44740..8d25ef48a2 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -233,6 +233,7 @@ typedef struct { /* put this in your own status structure as returned to application */ /* all fields are private to libxl_spawn_... */ pid_t intermediate; + int fd; char *what; /* malloc'd in spawn_spawn */ } libxl__spawn_starting; @@ -270,6 +271,7 @@ _hidden int libxl__confirm_device_model_startup(libxl__gc *gc, _hidden int libxl__detach_device_model(libxl__gc *gc, libxl__device_model_starting *starting); _hidden int libxl__wait_for_device_model(libxl__gc *gc, uint32_t domid, char *state, + libxl__device_model_starting *starting, int (*check_callback)(libxl__gc *gc, uint32_t domid, const char *state, diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index 6001c6f06f..c50b74109f 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -612,7 +612,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i hvm = libxl__domain_is_hvm(gc, domid); if (hvm) { - if (libxl__wait_for_device_model(gc, domid, "running", NULL, NULL) < 0) { + if (libxl__wait_for_device_model(gc, domid, "running", + NULL, NULL, NULL) < 0) { return ERROR_FAIL; } path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid); @@ -626,7 +627,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i pcidev->bus, pcidev->dev, pcidev->func); path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/command", domid); xs_write(ctx->xsh, XBT_NULL, path, "pci-ins", strlen("pci-ins")); - rc = libxl__wait_for_device_model(gc, domid, NULL, pci_ins_check, state); + rc = libxl__wait_for_device_model(gc, domid, NULL, NULL, + pci_ins_check, state); path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/parameter", domid); vdevfn = libxl__xs_read(gc, XBT_NULL, path); path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid); @@ -843,7 +845,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid, hvm = libxl__domain_is_hvm(gc, domid); if (hvm) { - if (libxl__wait_for_device_model(gc, domid, "running", NULL, NULL) < 0) { + if (libxl__wait_for_device_model(gc, domid, "running", + NULL, NULL, NULL) < 0) { return ERROR_FAIL; } path = libxl__sprintf(gc, "/local/domain/0/device-model/%d/state", domid); @@ -857,7 +860,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid, * device-model for function 0 */ if ( !force && (pcidev->vdevfn & 0x7) == 0 ) { xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem")); - if (libxl__wait_for_device_model(gc, domid, "pci-removed", NULL, NULL) < 0) { + if (libxl__wait_for_device_model(gc, domid, "pci-removed", + NULL, NULL, NULL) < 0) { LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Device Model didn't respond in time"); /* This depends on guest operating system acknowledging the * SCI, if it doesn't respond in time then we may wish to